-
Notifications
You must be signed in to change notification settings - Fork 929
(#2829) Adds Basic choco license Command #2847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is a WIP. Just commenting on a few things I noticed.
src/chocolatey/infrastructure.app/commands/ChocolateyLicenseCommand.cs
Outdated
Show resolved
Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyLicenseCommand.cs
Outdated
Show resolved
Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyLicenseCommand.cs
Outdated
Show resolved
Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyLicenseCommand.cs
Outdated
Show resolved
Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyLicenseCommand.cs
Outdated
Show resolved
Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyLicenseCommand.cs
Outdated
Show resolved
Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyLicenseCommand.cs
Outdated
Show resolved
Hide resolved
0febeca
to
f842a18
Compare
src/chocolatey/infrastructure.app/commands/ChocolateyLicenseCommand.cs
Outdated
Show resolved
Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyLicenseCommand.cs
Outdated
Show resolved
Hide resolved
13235a5
to
a22d727
Compare
@JPRuskin @corbob I have finished doing an overhaul of this PR. This changes that I have made include:
Let me know if either of you have any questions. |
src/chocolatey.tests/infrastructure.app/commands/ChocolateyLicenseCommandSpecs.cs
Outdated
Show resolved
Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyLicenseCommand.cs
Outdated
Show resolved
Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyLicenseCommand.cs
Outdated
Show resolved
Hide resolved
845a777
to
9c63d9b
Compare
@AdmiringWorm this should be ready for a final review now. I have fixed up the unit test that was failing as a result of the change that was made to the docs. |
This commit introduces a new command to Chocolatey, `license`. Currently, the functionality is limited, and it will only display details on the current license.
Not really the best thing to use as a test string. Let's use"abc" instead.
As a result of the overhaul of the wording for this command, we missed getting the pester test updated to use the correct wording. This PR addresses that oversight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
We'll ignore the docker build for now, it is exhibiting the same issues that happens from time to time when it comes with SSL connections, and since the ubuntu and windows build succeeded, we have the ones we want at the moment. |
Description Of Changes
This PR adds a delightful new command,
choco license
.It should offer read and write, but at the moment just offers a basic output of the current license.
It doesn't yet have tests, either.
Motivation and Context
This has been requested by awesome folk!
Testing
Change Types Made
Related Issue
Fixes #2829
Change Checklist